Skip to content

Force one token refresh after an API 401, bound HTTP timeouts, and keep reauth reachable - #319

Draft
pjordanandrsn wants to merge 2 commits into
binarydev:mainfrom
pjordanandrsn:claude/upstream-auth-reliability
Draft

pjordanandrsn wants to merge 2 commits into
binarydev:mainfrom
pjordanandrsn:claude/upstream-auth-reliability

Conversation

@pjordanandrsn

@pjordanandrsn pjordanandrsn commented Oct 29, 2026 •

Copy link
Copy Markdown

This contribution originated with @technorat2 in pjordanandrsn/ha-generac#4. Credit for the original implementation belongs to them: the forced refresh after a 401, the bounded timeouts, the timeout retry and the credential replacement on reauth are their work. I adapted it to current upstream, fixed a defect that blocked reauthentication, and added regression tests so these improvements can land in the main project. @technorat2 agreed on 2026-07-28 to move the work upstream (comment); I'm finishing that migration with full attribution.

@technorat2, I'd welcome your review, especially of anything I changed from your original.

Companion PR (independent, no ordering dependency): entity naming and telemetry, #320 (refs #284).

Problem

  • A 401 from the Mobile Link resource API currently raises SessionExpiredException straight away. Entities stay unavailable until the next poll, even when a single token refresh would have worked.
  • No API or token request sets a timeout, so a stalled connection holds a poll for aiohttp's 5-minute default.
  • Reauth/reconfigure merge new credentials into entry.data ({**entry.data, **entry_data}). Entries created by the retired cookie/password flows therefore keep password and session_cookie indefinitely.

Behavior changes

  • 401 → one forced refresh → one retry. GeneracAuth.force_refresh(rejected_token) mints a new access token. If a concurrent caller has already replaced the rejected token, it reuses that one instead of refreshing again, so concurrent callers can't spend the refresh token twice. A second 401 raises SessionExpiredException (→ UpdateFailed).
  • Correct exception semantics (fix beyond the original patch): the forced refresh runs outside the transport try/except. A revoked refresh token (invalid_grant, 400/401/403) therefore reaches the coordinator as InvalidCredentialsException → ConfigEntryAuthFailed → reauth. In the original patch it was wrapped as IOError and treated as transient, so reauth never started. A token-endpoint outage keeps its own type (UpdateFailed).
  • Bounded timeouts: 45 s total, 10 s connect, 30 s read. Applied to API GETs, token refresh, and the isolated login session, which covers the whole universal-login flow and code exchange.
  • One transport retry: a timeout or aiohttp.ClientConnectionError on a GET is retried once after 1 s, then raised as IOError. The transport retry and the 401 refresh retry each have their own single budget, so a request makes at most 3 attempts.
  • Coordinator: OSError (timeouts, connection failures, the IOError above) becomes a logged warning plus UpdateFailed instead of an "unexpected error" with a traceback, and the coordinator is marked offline as in the other failure branches.
  • Credential replacement: reauth/reconfigure drop password and session_cookie and keep any other keys.
  • Abandoned MFA flow (pre-existing on main): the config flow's async_remove is now a synchronous @callback that schedules aclose(). Home Assistant calls it without awaiting, so the old async def override never ran and a login session paused on the MFA form leaked. This also clears the GeneracFlowHandler.async_remove was never awaited warnings.
  • The resource server still uses Authorization: Bearer; nothing moves to DPoP (see the existing probe note in api.py).

Compatibility

  • No config entry version bump or migration. Entry data keeps the same three keys (username, refresh_token, dpop_pem); only stale legacy keys are removed, and only on reauth/reconfigure.
  • No changes to entities, the manifest, versions, CI or translations.

Regression tests added

tests/test_api.py, tests/test_auth.py, tests/test_coordinator.py, tests/test_config_flow.py (+24 tests). All network I/O is mocked; nothing contacts Mobile Link or Auth0.

  • valid token, no refresh; expired token refreshed (with timeout)
  • 401 → forced refresh → success (new Bearer on the retry)
  • persistent 401 after refresh → SessionExpiredException
  • revoked refresh token during the forced refresh → InvalidCredentialsException, explicitly not OSError
  • end to end: API 401 → real GeneracAuth/GeneracApiClient → 403 invalid_grant → ConfigEntryAuthFailed. This test fails on the original patch with OSError: … InvalidCredentialsException: revoked.
  • token-endpoint 5xx during the forced refresh keeps RuntimeError
  • timeout then success; repeated timeout; connection error then success; repeated connection error; timeout + 401 + success (separate budgets)
  • concurrent callers on expiry share one refresh; concurrent forced refreshes for the same rejected token share one refresh (verified to fail without the dedupe check); refresh-token rotation is persisted exactly once
  • failed refresh (403 and 500): neither the exception nor the logs contain the refresh token or the DPoP key
  • login session has the bounded timeout
  • reauth and reconfigure replace stale password / session_cookie
  • aborting a flow paused on the MFA form closes its login session (fails without the async_remove fix)

Validation (run locally, Python 3.14.8, homeassistant==2026.7.4, pytest-homeassistant-custom-component==0.13.348 from requirements.txt)

  • pytest: 99 passed, no warnings (baseline on main: 75 passed, 8 async_remove was never awaited warnings).
  • pre-commit run --all-files (check-yaml, end-of-file, trailing-whitespace, flake8, reorder-python-imports, black 23.3.0, prettier): all passed
  • python -m compileall custom_components: OK; git diff --check: clean
  • Merged together with the companion PR: 120 passed, no warnings
  • Not run locally: hassfest and HACS validation (CI runs both; this PR doesn't touch the manifest).

Commits and attribution

  1. Retry once after an API 401 and bound Mobile Link HTTP timeouts: authored by @technorat2, ported from their commits 7e3b067 and 40ae00b and adapted to current main. It deliberately preserves the original logic, including the defect fixed in commit 2.
  2. Keep reauth reachable when the forced refresh is rejected: the exception-semantics fix, refresh dedupe, connection-error retry, the async_remove fix and tests (Co-authored-by: technorat2).

If you squash-merge, GitHub keeps @technorat2 as a co-author.

Not carried over from the original PR

Limitations

  • Exercised only against mocks. The retry and timeout values (45/10/30 s, 1 s retry delay) are the original contribution's, not measured against Mobile Link.
  • A persistent 401 after a successful refresh stays UpdateFailed rather than reauth: the refresh token still works, so prompting for a password would not help.

🤖 Generated with Claude Code

Ported from pjordanandrsn/ha-generac#4 by @technorat2 (commits 7e3b067
"Improve authentication and entity reporting" and 40ae00b "Retry
transient Mobile Link timeouts"), adapted to current upstream:

- GeneracAuth.force_refresh(): mint a new access token when the
  resource server rejects the current one; get_endpoint() retries the
  request once with it.
- Bound every auth and API request (45 s total, 10 s connect, 30 s
  read) and retry a timed-out API GET once.
- Reauth/reconfigure replace stale credentials instead of merging them
  into the new entry data.
- Coordinator treats OSError (timeouts, connection failures) as a
  transient UpdateFailed rather than an unexpected error.

Adaptation notes: the login flow's dedicated session now carries the
timeout once instead of per request; the discarded-key set names the
credentials upstream's retired cookie/password flows actually stored
(password, session_cookie). The resource server Authorization scheme
stays Bearer.

Co-authored-by: Jordan Anderson <paul.jordan.anderson@gmail.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01K2RdXuPegztHA1uMG2fbtk
Follow-up to the ported 401 retry. get_endpoint() called force_refresh()
inside the transport `except Exception` block, so an InvalidGrantError
raised by the token endpoint was converted to InvalidCredentialsException
and then wrapped again as IOError. The coordinator treated that as a
transient failure and Home Assistant never started reauthentication.

- Move the forced refresh out of the transport try/except. A revoked
  refresh token now surfaces as InvalidCredentialsException
  (ConfigEntryAuthFailed). A token-endpoint outage keeps its own type
  (UpdateFailed).
- force_refresh(rejected_token): callers that were rejected with the same
  token share one refresh instead of each spending the refresh token.
- Retry transient connection failures (aiohttp.ClientConnectionError) as
  well as timeouts. Each request gets at most one transport retry and at
  most one refresh-and-retry.
- The coordinator marks itself offline on transient OSError failures, as
  every other failure branch does.
- Make the config flow's async_remove a synchronous callback that
  schedules aclose(). Home Assistant calls it without awaiting, so the
  async override never ran and a login session paused on the MFA form
  leaked when the flow was abandoned.

Tests cover a valid token, an expired token refreshed, 401 followed by a
successful refresh, a persistent 401, a revoked refresh token, a refresh
outage, a timeout then success, repeated timeouts and connection
failures, concurrent callers, refresh-token rotation, secret hygiene on
failed refreshes, the bounded timeouts, stale legacy credentials being
dropped on reauth/reconfigure, and an abandoned MFA flow closing its
login session. One end-to-end test goes from an API 401 through the real
GeneracAuth/GeneracApiClient to ConfigEntryAuthFailed.

Co-authored-by: technorat2 <technorat2@users.noreply.github.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01K2RdXuPegztHA1uMG2fbtk
@pjordanandrsn
pjordanandrsn force-pushed the claude/upstream-auth-reliability branch from 6d7d21d to 088e5ac Compare October 29, 2026 08:27
pjordanandrsn added a commit to pjordanandrsn/ha-generac that referenced this pull request Oct 29, 2026
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants